feat: add aclp alert channel API support - #997
Open
shkaruna wants to merge 7 commits into
Open
Conversation
…a/linodego into feat/aclp-alert-channels
shkaruna
marked this pull request as ready for review
July 28, 2026 04:41
shkaruna
requested review from
Copilot,
ezilber-akamai and
mawilk90
and removed request for
a team
July 28, 2026 04:41
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Linodego client support for ACLP Monitor alert notification channels, including CRUD operations and listing alert definitions attached to a channel, plus accompanying unit/integration test coverage and recorded fixtures.
Changes:
- Introduces
AlertChannelCreateOptions/AlertChannelUpdateOptionsand client methods:GetAlertChannel,CreateAlertChannel,UpdateAlertChannel,DeleteAlertChannel, andListAlertsForChannel. - Expands unit tests for alert channels to cover CRUD + list-alerts behavior with updated mock responses.
- Updates integration tests/fixtures to create and clean up alert channels during monitor alert definition flows, and adds new E2E fixtures for alert-channel CRUD and list-alerts.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| monitor_alert_channels.go | Adds alert-channel CRUD + list-alerts client methods and request/response types. |
| test/unit/monitor_alert_channels_test.go | Adds unit tests for alert channel CRUD and listing alerts; updates mock payloads. |
| test/integration/monitor_alerts_test.go | Updates integration tests to create/delete alert channels during alert-definition tests; adds alert-channel E2E tests. |
| test/integration/fixtures/TestMonitorAlertDefinition.yaml | Records additional interactions for user lookup + alert-channel creation/deletion during smoke flow. |
| test/integration/fixtures/TestMonitorAlertDefinition_CreateWithIdempotency.yaml | Updates fixture to record user lookup + alert-channel creation/deletion. |
| test/integration/fixtures/TestMonitorAlertDefinition_Clone.yaml | Updates fixture to record user lookup + alert-channel creation/deletion for clone flow. |
| test/integration/fixtures/TestMonitorAlertChannel_ListAlerts.yaml | New fixture for alert-channel list-alerts integration test. |
| test/integration/fixtures/TestMonitorAlertChannel_CRUD.yaml | New fixture for alert-channel CRUD integration test. |
Comments suppressed due to low confidence (7)
test/integration/monitor_alerts_test.go:110
channelIDis still declared in the package-level const block, but it is no longer referenced anywhere in theintegrationtest package after switching to creating channels dynamically. This will make thetest/integrationpackage fail to compile due to an unused identifier; please remove the stale constant (and its TODO) or reintroduce a usage.
test/integration/monitor_alerts_test.go:110- This test indexes
users[1]later when buildingUsernames, but it only checks the error fromListUsers. If the account has fewer than 2 users (common in record mode), this will panic. Add an explicit length assertion before indexing.
This issue also appears in the following locations of the same file:
- line 289
- line 412
- line 533
- line 620
test/integration/monitor_alerts_test.go:290
- This test indexes
users[1]later when buildingUsernames, but it only checks the error fromListUsers. If the account has fewer than 2 users (common in record mode), this will panic. Add an explicit length assertion before indexing.
test/integration/monitor_alerts_test.go:413 - This test indexes
users[1]later when buildingUsernames, but it only checks the error fromListUsers. If the account has fewer than 2 users (common in record mode), this will panic. Add an explicit length assertion before indexing.
test/integration/monitor_alerts_test.go:534 - This test indexes
users[1]when buildingUsernames, but it only checks the error fromListUsers. If the account has fewer than 2 users, this will panic. Add an explicit length assertion before indexing.
test/integration/monitor_alerts_test.go:621 - This test indexes
users[1]when buildingUsernames, but it only checks the error fromListUsers. If the account has fewer than 2 users, this will panic. Add an explicit length assertion before indexing.
test/unit/monitor_alert_channels_test.go:85 - The API/fixtures use
"type": "alerts-definitions"in thealertsblock (see this file's list response and integration fixtures), but the update response uses"alert-definitions"(missing the 's'). This inconsistency makes the unit fixture less representative and can hide issues.
"alerts": {
"alert_count": 0,
"type": "alert-definitions",
"url": "/monitor/alert-channels/10000/alerts"
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
59
to
+63
| "alerts": { | ||
| "url": "/monitor/alert-channels/123/alerts", | ||
| "type": "alerts-definitions", | ||
| "alert_count": 0 | ||
| }, | ||
| "created": "2024-01-01T00:00:00", | ||
| "updated": "2024-01-01T00:00:00", | ||
| "alert_count": 0, | ||
| "type": "alert-definitions", | ||
| "url": "/monitor/alert-channels/10000/alerts" | ||
| } |
Comment on lines
+109
to
+112
| // EmailChannelUpdateOptions represents email-specific update configuration for an alert channel. | ||
| type EmailChannelUpdateOptions struct { | ||
| Usernames []string `json:"usernames,omitzero"` | ||
| } |
Comment on lines
+114
to
+121
| // Alert represents an alert definition assigned to a notification channel. | ||
| type Alert struct { | ||
| ID int `json:"id"` | ||
| Label string `json:"label"` | ||
| ServiceType string `json:"service_type"` | ||
| Type string `json:"type"` | ||
| URL string `json:"url"` | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
This PR adds support for ACLP alert notification channels.
POST /monitor/alert-channels
GET /monitor/alert-channels/{id}
PUT /monitor/alert-channels/{id}
DELETE /monitor/alert-channels/{id}
GET /monitor/alert-channels/{id}/alerts
✔️ How to Test
Unit Tests
Run all the unit tests:
make test-unit
Run Monitor alert definition unit tests:
go test -v ./test/unit/... -run AlertChannel
Integration Tests
Play mode:
Run all integration tests (sets LINODE_FIXTURE_MODE="play"):
make test-int
Run alert channel integration tests:
make test-int TEST_ARGS='-run "AlertChannel"'
Record mode:
export LINODE_TOKEN=<my_token>
ENABLE_CLOUD_FW=false make TEST_ARGS='-run "TestMonitorAlertChannel_CRUD_E2E"' fixtures
ENABLE_CLOUD_FW=false make TEST_ARGS='-run "TestMonitorAlertChannel_ListAlerts"' fixtures